Repository navigation
Conversation
[Medium impact] The PR appears safe to merge; no blocking issues remain. SummaryThis PR separates timeline rows from page captures.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Mapped command] --> B{Changes the page?}
B -->|Yes| C[Capture page state]
B -->|No| D{Initial capture needed?}
D -->|Yes| C
D -->|No| E[Reuse earlier capture]
C --> F[Render timeline row]
E --> F
Reviews (2) · Last reviewed commit: "fix(trace): retry the initial frame only..." · Reviewed by Greptile |
| if (snap) { | ||
| upsertRichestSnapshot(ctx.actionSnapshots, snap) | ||
| } else if (initialFrame && !changesPage(cmd.command)) { | ||
| capturer.awaitingInitialFrame = true |
There was a problem hiding this comment.
Failed screenshot stops later attempts
If the first screenshot fails, captureActionSnapshot can still return a truthy object with screenshot: undefined. This branch leaves awaitingInitialFrame false, so later reads never retry. FrameSnapshotIndex skips that capture, and Selenium has no final capture to recover it. A read-only test can therefore finish without frames for its rows.
Clear the initial-frame flag only after a usable screenshot arrives. Apply the same check in Nightwatch and the service.
ReviewThe direction is right and the diff is small. Splitting "does this get a timeline row" from "does this get a capture" via I'd ask for one fix before merging and have one design question. Must fix1. The initial-frame retry never fires. Greptile's finding is correct, and it's slightly worse than it describes.
Suggested fix: gate on Should address2. The service's action rows now show a later moment (not mentioned in the PR). The service captures before a command and stamps the capture with the timestamp of the last page change. Take
The capture count is the same (one per change); only when it's taken moves. Showing the settled state may well be what we want, but it is a user-visible change. The "0 rows differing" table compares page text on suites that probably have no async transitions, so it wouldn't catch this. Please either state it in the changeset or check it on a suite with a loading state. Selenium and Nightwatch capture right after the command, so the adapters now differ here as well. 3. A late result from one test can flip the next test's flag (Selenium/Nightwatch). The capture promise isn't awaited. If test N's first capture resolves after test N+1 has already taken its initial frame, the retry branch sets Nits / questions
Generated by Claude Code |
Trace mode took a per-action capture (page source + screenshot, plus DOM on web) for reads and waits as well as actions, because the capture gate was mapCommandToAction, the timeline-rendering table. 5c54ae4 added the reads to that table so they would render as rows, which also made every read, and every poll of a waiting matcher, a capture. On iOS a capture is expensive; a reported suite made 187 page-source calls for 12 interactions and went from 3:10 to 7:10. ACTION_MAP's methods are now classified in a typed METHOD_EFFECT table (an unclassified method fails to compile) and changesPage() gates the capture in all three JS adapters. Reads keep their rows and inherit the latest earlier capture through claimAfter's fallback. The first traced command of each test is still captured so a test opening with reads has a frame. The WDIO service stamps its end-of-test capture at the test's last page change, otherwise that action's row resolves to the capture from before it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
captureActionSnapshot reports a failed screenshot probe as a snapshot without a screenshot, not as null, so the retry keyed on a falsy result never fired and a test that opened with reads could end up with no frame. The flag now stays armed until a capture carries a screenshot, under one rule in all three adapters, and is retried at most once per test so a driver that cannot take screenshots does not capture every read again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b012d0c to
14e1b41
Compare
What & why
In trace mode every adapter took a per-action capture (page source and screenshot, plus DOM/a11y on web) for read and wait commands as well as for actions. On iOS a capture is expensive, because WDA computes
visiblefor every node. One reported suite made 187getPageSourcecalls for 12 interactions and slowed from 3:10 to 7:10.The capture gate has always been
mapCommandToAction, the table that decides which commands render as timeline rows. 5c54ae4 (#247) added the reads (getText,is*,waitFor*,getTitle, …) toACTION_MAPso they would render as rows. From then on each of them was also captured, including every poll of a waiting matcher. One table was answering two questions.METHOD_EFFECTclassifies every trace method aschangesorreads.ACTION_MAPis typed against it, so an unclassified entry fails to compile.changesPage(command)is the new capture gate.changesPage. The first traced command of each test is also captured, so a test that opens with reads still has a frame. Read and wait rows keep rendering and inherit the latest earlier capture throughclaimAfter's existinglatestAtOrBeforefallback.captureActionSnapshotreports a failed screenshot as a snapshot with no screenshot rather than asnull. The retry is capped at one per test, so a driver that cannot take screenshots doesn't capture every read again. The rule is the same in all three adapters.Type of change
Packages touched
shared(types and contracts)core(framework-agnostic capture/reporting)elements(published element/snapshot API —@wdio/elements)service(WebdriverIO adapter)nightwatch-devtools(Nightwatch adapter)selenium-devtools(Selenium adapter)selenium-devtools-py(Selenium Python adapter)backend(server)app(UI)script(page-injected runtime)trace(Trace mode) (tests only)Notes for reviewers
Why isn't this in
core? The gate is one call per adapter, but each adapter captures at a different point: the service before the command, Selenium and Nightwatch after it. The shared part is the classification, and that lives inshared.Behavior change in the service. The service captures before a command, so an action's row now shows the page just before the next page-changing command, after any waits and reads in between. Before, it showed the page just before the first read that followed. Take
click(A) → waitForDisplayed → getText → click(C): A's row used to show the moment after the click, possibly mid-transition, and now shows the settled state. The number of captures per page change is unchanged; only their timing moves. Selenium and Nightwatch capture right after each command, so they are unaffected.Real runtime,
upstream/mainvs this branch, 3 runs each,DEVTOOLS_MODE=trace. "Rows differing" compares each row's resolved snapshot page text against the base run. These runs were measured before the retry fix, which only changes the screenshot-failure path.The Nightwatch wait after the submit click still resolves to
/secure: it inherits the click's own capture, which is taken after the click. Wall time on Chrome doesn't move because captures are cheap there.Checks: after the rebase onto
upstream/main,pnpm buildpasses,pnpm testreports 2859 passed and 3 skipped, andpnpm lintreports 0 errors. New tests cover:null;Not measured: iOS, the case that motivated this. A rerun of the reporting suite on this build would confirm the gain.
Known limits:
describe/it: the plugin's per-test hook fires once per module, so the first-command capture happens once per module.browser.url()with no argument: this getter still counts as a page change.executecounts as a page change. That includes Appiummobile:reads, which only read.Screenshots / recordings
n/a (no UI change)
🤖 Generated with Claude Code